Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

API: Export ActionCommands() and ActionFlags() for cobra command #888

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

maxlandon
Copy link
Contributor

Changes

This PR simply exports the ActionCommands() and ActionFlags() which both receive
a cobra command as argument.

Reasons

The reason is simple: more and more people will try and succeed (some already do)
in binding/executing cobra commands dynamically, for that very reason might want
to beneficiate from the usual completion utilities, like caching completions.

An example would be a tool of which a significant part of its command tree might
available/unavailable based on different conditions, and rather than having to
always query those on the wire, might just use cached command completions.

Included commits

  • Add a CacheF function force the engine to refresh within timeout if needed.
  • Export cobra subcommand/flag completion.

@rsteube
Copy link
Member

rsteube commented Aug 13, 2023

This is something I also had on my mind for quite a while but haven't done so far.
I can't remember the exact reason but it should mostly be because there were still
lots of changes happening so keeping them private was better at the time.

What I am thinking of now though, and what would oppose doing this, is that ActionCommands
and ActionFlags are rather some dumb low-level actions that are missing context.
So you can't pass it as PositionalAny as ActionFlags for example wouldn't know if a flag
was already set (unless the flagset was already parsed) or if a value of a flag should be completed.
ActionExecute is mostly a better choice as it includes traverse which handles all of this.

Might still be useful though anyway, so I'll give it some more thought.

@maxlandon
Copy link
Contributor Author

Thanks for these explanations.
I had not thought of this PositionalAny problem myself.

@rsteube rsteube mentioned this pull request Oct 3, 2023
@maxlandon
Copy link
Contributor Author

Pinging on this to know if there is a reason why the ActionFlags has not been exposed ?

Asking this because the initial intent was to give some way to dynamically complete commands/flags, should consumer libraries need it.

@rsteube
Copy link
Member

rsteube commented Dec 9, 2023

Pinging on this to know if there is a reason why the ActionFlags has not been exposed ?

Asking this because the initial intent was to give some way to dynamically complete commands/flags, should consumer libraries need it.

ActionFlags is a bit more complicated and entangled with with other code. I also have no concrete use case for it in sight at the moment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants